Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FOCUS #557: Preview - Refinement of FOCUS Columns Normative Requirements #664

Open
wants to merge 17 commits into
base: working_draft
Choose a base branch
from

Conversation

ijurica
Copy link
Contributor

@ijurica ijurica commented Dec 10, 2024

This PR serves solely as a preview and discussion base and is not intended for merging into the working draft.

It includes both the original version and the candidate version of normative requirements across all existing columns to facilitate a clear comparison (before/after) for review and feedback.

The goal of this PR is to provide a comprehensive context for discussing potential updates to normative requirements across the specification. It aims to ensure that the impact of proposed changes on all column definitions is thoroughly reviewed and debated.

Please use this PR as a collaborative tool, keeping in mind that it is not finalized content and should not be merged into the working draft.


Instructions for Reviewing and Providing Feedback

If you wish to contribute to the refinement of Normative Requirements, please follow the steps below to review the current progress and provide your feedback.

1. Select Columns to Review:

  • In this spreadsheet, 24.12.07 Normative Requirements Cookbook, on the Overview sheet, you’ll find a list of FOCUS Columns requiring review.

  • As a first step, select one or more columns to validate and add your name to the "Assignee" column in the sheet (please focus on those marked with Initial Pass status).

2. Review proposed changes and provide feedback for the specific column you’ve selected

  • If you have concerns or suggestions related to the proposed changes for that column’s Normative Requirements, please add comments or provide alternative proposal using GitHub comments or suggestions to the specific file/change.
  • If you agree with the proposed changes for that column, approve them by adding an explicit approval using GitHub comment to the corresponding file/change.
  • If you identify any discrepancies or ambiguities that need further discussion (either in a meeting or offline), please flag them, and we'll add them to the list of Discussion Topics.

Notes:

  • While feedback on the overall PR is welcome, please ensure that feedback and approvals are provided at the individual column level for the columns you’ve selected.
  • At this stage, we are primarily seeking feedback on columns marked with the status Initial Pass, where alternative Normative Requirements have been proposed (provided in this PR). By January 14th, alternative proposals will also be added for the remaining columns.
  • This PR includes both the original version and a new candidate version of the Normative Requirements specifications for multiple FOCUS columns. It is not intended to be merged into the working draft. Instead, it is meant to support collaborative efforts in consolidating and improving the Normative Requirements for all FOCUS columns

@ijurica ijurica requested a review from a team as a code owner December 10, 2024 13:04
* AvailabilityZone is RECOMMENDED to be present in a [*FOCUS dataset*](#glossary:FOCUS-dataset) when the provider supports deploying resources or services within an *availability zone*.
* If present, the column MUST conform to the following additional requirements:
* AvailabilityZone MUST be of type String.
* AvailabilityZone MUST conform to [String Handling](#stringhandling) requirements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* AvailabilityZone MUST conform to [String Handling](#stringhandling) requirements.

Discussed in Dec 10 TF1. Propose postponing adding such requirements until lower-hanging fruit of consistency is collected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have missed something. Why is linking attributes contentious?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talking to Irena - she suggested postponing this due to provider conformance concerns.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we discuss this in the TF-1 meeting. This sheet can serve as the basis for our discussion.

* RegionId MUST be of type String.
* RegionId MUST conform to [String Handling](#stringhandling) requirements.
* RegionId MUST NOT be null when a *resource* or *service* is operated in or managed from a distinct region by the Provider.
* RegionId MAY be null when a *resource* or *service* is not restricted to an isolated geographic area.
Copy link

@davehatcher davehatcher Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the previous line, with a MUST NOT be NULL condition - we say 'operated in or managed from a distinct region'
in this line where it MAY be NULL we say 'not restricted to an isolated geographical area'.

would the 2nd line be clearer as follows:

Suggested change
* RegionId MAY be null when a *resource* or *service* is not restricted to an isolated geographic area.
* RegionId MAY be null when a *resource* or *service* is not operated in or managed from a distinct region.

Copy link
Contributor Author

@ijurica ijurica Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current version (using identical wording in both requirements):

  • RegionId MUST NOT be null when a resource or service is operated in or managed from a distinct region.
  • RegionId MAY be null when a resource or service is operated in or managed from a distinct region.

Suggestions:

  • Add 'region' as an additional term in the glossary.

* AvailabilityZone is RECOMMENDED to be present in a [*FOCUS dataset*](#glossary:FOCUS-dataset) when the provider supports deploying resources or services within an *availability zone*.
* If present, the column MUST conform to the following additional requirements:
* AvailabilityZone MUST be of type String.
* AvailabilityZone MUST conform to [String Handling](#stringhandling) requirements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have missed something. Why is linking attributes contentious?

The AvailabilityZone column adheres to the following requirements:

* AvailabilityZone is RECOMMENDED to be present in a [*FOCUS dataset*](#glossary:FOCUS-dataset) when the provider supports deploying resources or services within an *availability zone*.
* If present, the column adheres to the following additional requirements:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Personally, I would rather not have this line and keep all requirements at the root level. The requirements are all valid whether the column is present or not, so I'm not sure what the value of nesting these requirements is.

@@ -2,6 +2,17 @@

The [*billed cost*](#glossary:billed-cost) represents a charge serving as the basis for invoicing, inclusive of the impacts of all reduced rates and discounts while excluding the [*amortization*](#glossary:amortization) of relevant purchases (one-time or recurring) paid to cover future eligible charges. This cost is denominated in the [Billing Currency](#billingcurrency). The Billed Cost is commonly used to perform FinOps capabilities that require cash-basis accounting such as cost allocation, budgeting, and invoice reconciliation.

---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we have a requirement that these lines be here? Do we require them to have specific spacing before/after? I don't mind them. I would just ask that they have empty lines before and after to align to existing markdown linting rules. (Although, I don't think we've implemented any yet.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines are just temporary separators surrounding the candidate versions of the normative requirements (helping us to more easily spot the before/after differences).

* ChargeClass MUST be present in a [*FOCUS dataset*](#glossary:FOCUS-dataset).
* ChargeClass MUST be of type String.
* ChargeClass MUST be null when the row does not represent a correction or when it represents a correction within the current *billing period*.
* When the row represents a correction to a previously invoiced *billing period*, the following applies:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "If"? Also aligning to a previous suggestion...

Suggested change
* When the row represents a correction to a previously invoiced *billing period*, the following applies:
* If the row represents a correction to a previously invoiced *billing period*, the column adheres to the following additional requirements:

specification/columns/commitmentdiscountcategory.md Outdated Show resolved Hide resolved
specification/columns/commitmentdiscountname.md Outdated Show resolved Hide resolved
specification/columns/commitmentdiscountstatus.md Outdated Show resolved Hide resolved
specification/columns/pricingcategory.md Outdated Show resolved Hide resolved
* ServiceSubcategory MUST NOT be null.
* ServiceSubcategory MUST be one of the allowed values.
* ServiceSubcategory MUST have one and only one parent ServiceCategory as specified in the allowed values below.
* Though a given *service* can have multiple purposes, each *service* SHOULD have one and only one ServiceSubcategory that best aligns with its primary purpose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a confusing requirement. We have allowed values, so it should be a MUST. If this is context on the allowed values requirement, I would rather see it nested under that and remove the "SHOULD".

Copy link
Contributor Author

@ijurica ijurica Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to Michael's comment,

  • I suggest moving this requirement to the ServiceName column definition, as it is one of the normative requirements for ServiceName.
  • Furthermore, considering that ServiceSubcategory is currently RECOMMENDED, I propose adding two explicit and easily verifiable requirements to the ServiceName column: one defining its relationship with ServiceSubcategory, and another for its relationship with ServiceCategory.
  • Additionally, I would like to establish a consistent pattern for column relationship-related requirements.

PREFERRED PATTERN:

  • XY MUST be associated with one and only one parent MN ...

SkuPriceId:

BEFORE:

  • SkuPriceId MUST be associated with one and only one SkuId, except ...

AFTER:

  • SkuPriceId MUST be associated with one and only one parent SkuId, except ...

ServiceCategory:

The Service Category is the highest-level classification of a service based on the core function of the service. Each service should have one and only one category that best aligns with its primary purpose.

ServiceSubcategory:

BEFORE:

The Service Subcategory is a secondary classification of the Service Category for a service based on its core function.
...

  • ServiceSubcategory MUST have one and only one parent ServiceCategory as specified in the allowed values below.
  • Though a given service can have multiple purposes, each service SHOULD have one and only one ServiceSubcategory that best aligns with its primary purpose

AFTER:

The Service Subcategory is a secondary classification of the Service Category for a service based on its core function.
...

  • ServiceSubcategory MUST be associated with one and only one parent ServiceCategory as specified in the allowed values below.

ServiceName (for discussion in a meeting):

BEFORE:

A service represents an offering that can be purchased from a provider...
...
The Service Name is a display name for the offering that was purchased...

AFTER:

A service represents an offering that can be purchased from a provider...
...
The Service Name is a display name for the offering that was purchased...
...

  • Each service SHOULD be associated with one and only one parent ServiceCategory that best aligns with its primary purpose.
  • Each service SHOULD be associated with one and only one parent ServiceSubcategory that best aligns with its primary purpose if ServiceSubcategory is present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @ijurica on this

@ijurica
Copy link
Contributor Author

ijurica commented Jan 7, 2025

Instructions for Reviewing and Providing Feedback

If you wish to contribute to the refinement of Normative Requirements, please follow the steps below to review the current progress and provide your feedback.

1. Select Columns to Review:

  • In this spreadsheet, 24.12.07 Normative Requirements Cookbook, on the Overview sheet, you’ll find a list of FOCUS Columns requiring review.

  • As a first step, select one or more columns to validate and add your name to the "Assignee" column in the sheet (please focus on those marked with Initial Pass status).

2. Review proposed changes and provide feedback for the specific column you’ve selected

  • If you have concerns or suggestions related to the proposed changes for that column’s Normative Requirements, please add comments or provide alternative proposal using GitHub comments or suggestions to the specific file/change.
  • If you agree with the proposed changes for that column, approve them by adding an explicit approval using GitHub comment to the corresponding file/change.
  • If you identify any discrepancies or ambiguities that need further discussion (either in a meeting or offline), please flag them, and we'll add them to the list of Discussion Topics.

Notes:

  • While feedback on the overall PR is welcome, please ensure that feedback and approvals are provided at the individual column level for the columns you’ve selected.
  • At this stage, we are primarily seeking feedback on columns marked with the status Initial Pass, where alternative Normative Requirements have been proposed (provided in this PR). By January 14th, alternative proposals will also be added for the remaining columns.
  • This PR includes both the original version and a new candidate version of the Normative Requirements specifications for multiple FOCUS columns. It is not intended to be merged into the working draft. Instead, it is meant to support collaborative efforts in consolidating and improving the Normative Requirements for all FOCUS columns

* If present, the column MUST conform to the following additional requirements:
* ResourceId MUST be of type String.
* ResourceId MUST conform to [String Handling](#stringhandling) requirements.
* ResourceId MUST be null when a charge is not related to a *resource*.
Copy link

@smokemonster99 smokemonster99 Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like two spaces after when a charge. Otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double space resolved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

specification/columns/subaccountid.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after edit is made to correct doublespace

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

* ServiceSubcategory MUST NOT be null.
* ServiceSubcategory MUST be one of the allowed values.
* ServiceSubcategory MUST have one and only one parent ServiceCategory as specified in the allowed values below.
* Though a given *service* can have multiple purposes, each *service* SHOULD have one and only one ServiceSubcategory that best aligns with its primary purpose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @ijurica on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@ijurica ijurica self-assigned this Jan 16, 2025
@ijurica ijurica added the 1.2 Agreed scope for release 1.2 label Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.2 Agreed scope for release 1.2
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

7 participants